==69600==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f6776e2a5d6 bp 0x000000000000 sp 0x7fff65042010 T0) ==69600==The signal is caused by a READ memory access. ==69600==Hint: address points to the zero page. #0 0x7f6776e2a5d5 in _$LT$style..rule_collector..RuleCollector$LT$$u27$a$C$$u20$$u27$b$C$$u20$E$C$$u20$F$GT$$GT$::collect_normal_rules_from_containing_shadow_tree::hc6465809d2630c8f src/servo/components/style/rule_collector.rs:222:16 #1 0x7f6776e2a5d5 in _$LT$style..rule_collector..RuleCollector$LT$$u27$a$C$$u20$$u27$b$C$$u20$E$C$$u20$F$GT$$GT$::collect_all::ha1550e657e348e26 src/servo/components/style/rule_collector.rs:386 #2 0x7f6776e2a5d5 in style::stylist::Stylist::push_applicable_declarations::h66da088a9d41b41b src/servo/components/style/stylist.rs:1125 #3 0x7f6776e2a5d5 in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::match_primary::h0c2e888ee2041b0a src/servo/components/style/style_resolver.rs:435 #4 0x7f6776edf64d in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::resolve_primary_style::hf1e863802eba5fa6 src/servo/components/style/style_resolver.rs:162:30 #5 0x7f6776edf64d in style::traversal::resolve_style::h2564578a9b75e517 src/servo/components/style/traversal.rs:359 #6 0x7f6776edf64d in Servo_ResolveStyleLazily src/servo/ports/geckolib/glue.rs:4931 #7 0x7f67706384d0 in mozilla::ServoStyleSet::ResolveStyleLazilyInternal(mozilla::dom::Element*, mozilla::CSSPseudoElementType, mozilla::StyleRuleInclusion) src/layout/style/ServoStyleSet.cpp:1395:5 #8 0x7f677069d0ba in nsComputedDOMStyle::UpdateCurrentStyleSources(bool) src/layout/style/nsComputedDOMStyle.cpp:981:7 #9 0x7f677069b662 in nsComputedDOMStyle::GetPropertyValue(nsTSubstring<char16_t> const&, nsTSubstring<char16_t>&) src/layout/style/nsComputedDOMStyle.cpp:450:3 #10 0x7f676ba29528 in GetPropertyValue src/layout/style/nsICSSDeclaration.h:91:10 #11 0x7f676ba29528 in mozilla::dom::CSSStyleDeclaration_Binding::getPropertyValue(JSContext*, JS::Handle<JSObject*>, nsICSSDeclaration*, JSJitMethodCallArgs const&) src/obj-firefox/dom/bindings/CSSStyleDeclarationBinding.cpp:289 #12 0x7f676d3347b4 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3317:13 #13 0x2f35387b5ebf (<unknown module>)

Huh, this is a really recent regression, and I still don't know how it can happen... I can't repro it with a build from yesterday, has anything suspicious landed today?

https://drafts.csswg.org/cssom/#dom-window-getcomputedstyle says: > If elt is connected, part of the flat tree, and its shadow-including root... WebKit and Blink already do this, and we do it already except for cross-document situations, where we can end up with a PresShell even if GetPresShellForContent returns null. The style system should be able to rely on ShadowRoots having a non-null shadow host.

This only fixes the style crash, but not the root of the problem. Let's use bug 1511130 for that.

See D13472 for spec quotes and such. Other browsers don't allow getting computed styles in disconnected subtrees and we agreed to follow suit (it does make sense because when you're not on the flat tree it's not defined what you're supposed to inherit from, specially in presence of Shadow DOM). Also, it allows the style system to rely on the DOM being in a sane state.

This probably deserves a comment as of why it belongs to this bug. This patch series caused a single, reproducible timeout on browser_ext_themes_toolbars.js, where the transitionend event it awaits for stops triggering. I got fascinated by it and I decided to poke around it in rr instead of just removing the await line, and here's what's going on. In the previous implementation of _sanitizeCSSColor, we were not flushing style because of the optimization bug 1363805 introduced (which wasn't supposed to deal with out-of-document elements, but it accidentally did so). In any case, the fact that we were not flushing style in _sanitizeCSSColor caused us to flush style sometime later when the lwtheme attribute was already set up, and thus the selector in here matched: https://searchfox.org/mozilla-central/rev/cfaa5a1d48d6bc6552199e73004ecb05d0a9c921/browser/themes/shared/browser.inc.css#40 And thus caused the transition rule to apply at a time where the background-color change happened. Now we were flushing on getComputedStyle on every call, and in the most inefficient way possible (changing a custom property on the root before each property change, which causes us to restyle the whole document to propagate it down to all descendants). Furthermore, we were flushing style at a time where the lwtheme attribute change had not yet happened, and thus when the background-color changed, there was no transition rule applicable, and the transition didn't fire. This patch changes LightweightThemeConsumer to avoid restyling the whole document over and over. Also, while at it I realized that you could fool the sanitizer with !important in an experiment stylesheet or with other !important rule in the page really. It's not clear why you'd do that, but it may be worth to just making that function completely sound, so I did that and added a test for it.

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/e601a2fbd077 Never return styles for disconnected elements. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/d2d3e7a822b5 Fix / update tests. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/e03afbff55f3 Fix LightweightThemeConsumer's use of getComputedStyle. r=jaws,mconley https://hg.mozilla.org/integration/mozilla-inbound/rev/5f5d1c50a342 Improve performance of LightweightThemeConsumer when setting properties, and also avoid _sanitizeCSSColor from getting fooled. r=jaws

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14401 for changes under testing/web-platform/tests

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/65e99e6399b9 Fix lint and OSX-only test failure on a CLOSED TREE.

Backed out for browser chrome failures on browser_selectpopup_colors.js. Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=browser%2Cchrome&fromchange=5f5d1c50a34213414f3f6a1f05b575e4f2176daf&tochange=90189bd84466d74e64d4eb2a33138152e84b241c&selectedJob=215755953 Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=215767528&repo=mozilla-inbound&lineNumber=1940 Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/90189bd84466d74e64d4eb2a33138152e84b241c 16:53:05 INFO - TEST-PASS | browser/base/content/test/forms/browser_selectpopup_colors.js | Item 3 has correct background color - 16:53:05 INFO - Buffered messages finished 16:53:05 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/forms/browser_selectpopup_colors.js | Item 4 should not have any custom option styling - 16:53:05 INFO - Stack trace: 16:53:05 INFO - chrome://mochikit/content/browser-test.js:test_ok:1292 16:53:05 INFO - chrome://mochitests/content/browser/browser/base/content/test/forms/browser_selectpopup_colors.js:testOptionColors:213 16:53:05 INFO - chrome://mochitests/content/browser/browser/base/content/test/forms/browser_selectpopup_colors.js:testSelectColors:290 16:53:05 INFO - chrome://mochitests/content/browser/browser/base/content/test/forms/browser_selectpopup_colors.js:test_colors_applied_to_popup_items:312 16:53:05 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1093 16:53:05 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1084 16:53:05 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:982 16:53:05 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803 16:53:05 INFO - Not taking screenshot here: see the one that was previously logged

Upstream PR was closed without merging

I missed the failure in browser_selectpopup_colors.js since it doesn't run on Linux. Fix the getComputedStyle usage in that code by using getDefaultComputedStyle, which is what it really wants. Also, do a bit of cleanup while at it: uaBackgroundColor was unused, and uaColor was wrong (we don't override the ua color of the <option> element, it just inherits, so it's the same as the <select> color, and that's what we were comparing it against anyway).

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/4df286b234b3 Never return styles for disconnected elements. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef293b90887 Fix / update tests. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/a99600391704 Fix LightweightThemeConsumer's use of getComputedStyle. r=jaws,mconley https://hg.mozilla.org/integration/mozilla-inbound/rev/d23c9c3e1566 Improve performance of LightweightThemeConsumer when setting properties, and also avoid _sanitizeCSSColor from getting fooled. r=jaws https://hg.mozilla.org/integration/mozilla-inbound/rev/daee82295b3c Fix getComputedStyle usage of SelectChild. r=jaws,mconley

Err, last commit shouldn't have landed with r=jaws, whoops... Anyway, he said on IRC he was fine as long as that was pointed out in the bug.

Backed out 5 changesets (bug 1511138) for causing eslint failure on SelectChild.jsm CLOSED TREE Backout revision: https://hg.mozilla.org/integration/mozilla-inbound/rev/5fcc6bd202c45914bdbbfcf9a8e3f3dde23c2870 Failed push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=215956522&revision=daee82295b3c81d57991a7db1b6851dc7bc8d963 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=215955742&repo=mozilla-inbound :emilio Please take a look.

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/7159c6b7e745 Never return styles for disconnected elements. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/1879534289a4 Fix / update tests. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/fa5967565fc6 Fix LightweightThemeConsumer's use of getComputedStyle. r=jaws,mconley https://hg.mozilla.org/integration/mozilla-inbound/rev/a3a2ea1539b8 Improve performance of LightweightThemeConsumer when setting properties, and also avoid _sanitizeCSSColor from getting fooled. r=jaws https://hg.mozilla.org/integration/mozilla-inbound/rev/f228599d5992 Fix getComputedStyle usage of SelectChild. r=mconley

Upstream PR was closed without merging

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

https://hg.mozilla.org/mozilla-central/rev/7159c6b7e745 https://hg.mozilla.org/mozilla-central/rev/1879534289a4 https://hg.mozilla.org/mozilla-central/rev/fa5967565fc6 https://hg.mozilla.org/mozilla-central/rev/a3a2ea1539b8 https://hg.mozilla.org/mozilla-central/rev/f228599d5992

Upstream PR merged